Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sidekiq, locks and invalidation #443

Merged
merged 2 commits into from
May 16, 2024

Conversation

adamruzicka
Copy link
Contributor

See commit messages

Dynflow uses locks placed into dynflow_coordinator_records table for two
reasons. One is to ensure certain things (delay planed watcher, world
invalidation and so on) are globally unique, the other is to ensure any
exceution plan is being worked on in only one world.

Sidekiq shutdown raises an exception in the executing thread, which then
propagates up the stack, executing all ensure blocks encountered along
the way. This could lead to the per-execution-plan kind of locks getting
unlocked prematurely. The originally reported issue was that execution
plans were being left in planning, without having a relevant planning
lock left in the database and therefore not being picked up during world
invalidation.

Furthermore, because the way Sidekiq performs shutdowns is a half-clean
shutdown, world termination hooks are being called, which prune all
locks belonging to the terminating world. This is nice for the
globaly unique kind of locks, but causes an issue for the
per-execution-plan kind of locks.

This change ensures that the per-execution-plan locks are left around on
world termination so that they can be dealt with properly during
invalidation, where we actually have time to do something with them.
World invalidation previously relied on world-level lock being present.
All locks belonging to the world which left that lock around would then
be invalidated. If there were any orphaned locks, those would be just
deleted.

With this change, we do proper invalidation for orphaned locks.
@adamruzicka adamruzicka merged commit 5e479a7 into Dynflow:master May 16, 2024
6 of 7 checks passed
@adamruzicka adamruzicka deleted the lock-everything branch May 16, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant